Skip to content

Remove dead code: unused gems and locale keys#52

Merged
dadachi merged 1 commit intophase1-rails-apifrom
dead-code-removal
Apr 29, 2026
Merged

Remove dead code: unused gems and locale keys#52
dadachi merged 1 commit intophase1-rails-apifrom
dead-code-removal

Conversation

@dadachi
Copy link
Copy Markdown
Contributor

@dadachi dadachi commented Apr 29, 2026

Summary

Removes verified-unreferenced gems and locale keys. Every removal was confirmed with a grep -rn across app/, lib/, config/ (and the gem source for the locale keys) that found no usage outside the definition site.

Removed

Gems

  • jbuilder ~> 2.14 — no .jbuilder view files exist; all responses go through jsonapi-serializer. Adds 1 transitive dep gone.
  • inline_svg ~> 1.10 — no inline_svg helper calls anywhere in app/views/ or controllers.

Locale keys

  • forgot_your_password
  • send_me_reset_password_instructions
  • api.shopkeeper.accounts.owner_required
  • api.shopkeeper.accounts.admin_required

Considered and kept

  • validate_sign_up_params / validate_account_update_params overrides in ShopkeeperAuth::RegistrationsController — initially looked dead (the subclass declares them but never calls them). Verified in the devise_token_auth 1.2.6 source: the parent class wires both as before_action on :create / :update, so the overrides DO get called when validation fails. Kept.
  • Other unused-looking locale keys (hello, welcome, change_my_password, change_your_password, confirm_new_password, new_password) — welcome/hello and the password form keys are referenced in views and mailer templates. Kept.

Test plan

  • bin/rails test (398 runs, 815 assertions, 0 failures)
  • bin/rubocop clean (227 files, 0 offenses)
  • bundle exec erb_lint --lint-all clean (14 files, 0 errors)
  • bin/brakeman clean (0 warnings)
  • bundle install cleanly removes both gems from Gemfile.lock
  • CI passes on the PR branch

🤖 Generated with Claude Code

Verified-unreferenced removals (every grep below ran across
app/, lib/, config/ and matched only the definition site):

- gem "jbuilder" — no .jbuilder views, no JBuilder usage; all
  responses go through jsonapi-serializer
- gem "inline_svg" — no inline_svg helper calls anywhere
- locale `forgot_your_password` — no callers
- locale `send_me_reset_password_instructions` — no callers
- locale `api.shopkeeper.accounts.owner_required` — no callers
- locale `api.shopkeeper.accounts.admin_required` — no callers

Kept `validate_sign_up_params` / `validate_account_update_params`
overrides in RegistrationsController despite looking unused — the
parent class wires them up via `before_action` (verified in the
gem source).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dadachi dadachi merged commit 12e666c into phase1-rails-api Apr 29, 2026
3 checks passed
@dadachi dadachi deleted the dead-code-removal branch April 29, 2026 22:59
dadachi added a commit that referenced this pull request May 2, 2026
* Add substrate v2 overview and phase 1 Rails API checklist

* Clarify ItemTag policy uses Shop permissions (no separate ItemTag perms)

* Remove NFC scan routes and display namespace

* Remove display namespace

* Remove static controller, scan actions, and root route

* Refactor ItemTag schema: rename queue_number, add description/position, remove queue-specific fields

* Refactor ItemTag model for generic CRUD

* Simplify Shop model: replace queue auto-generation with single sample item

Removes queue-specific Shop methods (latest_completed_item_tag,
create_default_item_tags!, reset!, full_reload_entire_page) and
replaces the after_create callback with create_sample_item_tag,
which generates one generic 'Sample' ItemTag for first-run UX.

Cascades caller cleanup:
- shops_controller#reset action removed
- delete :reset route removed
- ShopPolicy#reset? removed
- lib/tasks/shop.rake (only task was create_default_item_tags) deleted

* Update ItemTag controller for new schema

- Sort by (position, name) instead of queue_number
- Strip already_completed cache-purge hack from #complete
- Rename #reset action to #idle (matches AASM event); route updated
- Strong params: permit name, description, position, state

* Inline complete/idle state transitions, drop tag-named wrapper methods

Removes Shop#complete_tag! and ItemTag#reset! (both carried queue-era
'_tag' residue and 'reset' semantics). Controller actions now call
AASM's generated complete!/idle! events directly with completed_by /
completed_at set inline. This keeps method names domain-agnostic so
nativeapptemplate-agent's identifier rename (e.g. ItemTag to Todo)
doesn't need to rewrite method bodies.

* Update serializers for new ItemTag schema

ItemTagSerializer:
- Drop queue_number, scan_state, customer_read_at, already_completed
- Add name, description, position

ShopSerializer:
- Drop scanned_item_tags_count (referenced removed scan_state)
- Drop display_shop_server_path (display route removed)

* Update ItemTag policy: 2-tier admin/member, add idle, drop reset

- Remove references to senior_manager?/junior_manager?/senior_member?/junior_member? predicates
- Both admin and member can perform all ItemTag operations (collaborative model)
- Rename reset? to idle? to match controller action

Note: member? predicate doesn't yet exist; defined when AccountsShopkeeper::ROLES is updated in Step 13.

* Update madmin resource for new ItemTag schema

* Redesign roles (admin/member) and permissions (generic CRUD)

Permissions: 6 generic CRUD primitives — create_shops, update_shops,
delete_shops, update_organizations, invitation, read_data.
Removes 5 queue-specific perms (manage_tags, write_info_to_tags,
reset_all_tags, complete_or_reset_tags, show_tag_info).

Roles: 2 tiers — admin, member. Removes 5 queue-operator tiers
(senior_manager, junior_manager, senior_member, junior_member, guest).

Mappings (collaborative SaaS / Notion-style):
- admin: all 6 permissions
- member: create_shops, update_shops, delete_shops, read_data
  (no update_organizations or invitation)

Applied identically to all 4 envs (development, test, staging, production).
A fresh db rebuild + db:seed_fu is required to drop orphan rows from
prior fixture loads.

* Collapse roles to admin/member tier across model and madmin

AccountsShopkeeper::ROLES = [:admin, :member]. AccountsInvitation
inherits via constant aliasing, so both models drop the 4 queue-era
intermediate roles (senior_manager, junior_manager, senior_member,
junior_member) and 'guest'.

Madmin resources (AccountsShopkeeperResource, AccountsInvitationResource)
updated to expose only :admin and :member attributes.

Rolified concern is generic (iterates ROLES) so no change needed there.
Existing rows' 'roles' jsonb column may carry orphan keys for removed
roles — these are silently ignored since the model only reads ROLES.

* Update locales for new ItemTag schema and item-tag UI label

- Rename 'number tag(s)' → 'item tag(s)' in user-visible strings
  (aligns with substrate v2 UI label rename in overview §2.4)
- Replace activerecord.attributes.item_tag.queue_number with
  name / description / position
- Drop queue_number format/uniqueness error keys (validations gone)

No removed permission/role tag refs existed in en.yml; no ja.yml present.

* Update model tests for refactored schema and 2-tier roles

- ItemTagTest: rewrite around name/description/position; drop
  scan_tag!/complete_tag!/reset!/scan!/unscan! tests and queue_number
  format/uniqueness coverage; assert names may duplicate across and
  within shops
- ShopTest: replace default-item-tags / queue_number /
  latest_completed_item_tag / Shop#reset! tests with create_sample_item_tag
  success path and a stubbed-failure path covering the rescue branch
- AccountsShopkeeperTest: collapse per-tier permission/role-helper tests
  to admin and member only
- AccountsInvitationTest, AccountTest, RoleTest, ShopkeeperTest:
  rename junior_member/guest references to member

* Update serializer tests for refactored ItemTag/Shop attributes

- ItemTagSerializerTest: drop scan_state/customer_read_at/already_completed
  assertions; switch the complete-flow test to the inlined controller
  pattern (set completed_by/at then call AASM complete!)
- ShopSerializerTest: drop scanned_item_tags_count and
  display_shop_server_path tests (attributes removed)
- AccountSerializerTest, AccountsShopkeeperSerializerTest,
  AccountsInvitationSerializerTest: rename old-tier role refs to member

* Update policy tests for admin/member roles and idle action

- ItemTagPolicyTest: collapse per-tier tests to admin+member; rename
  reset? cases to idle?
- ShopPolicyTest: drop reset? tests (action removed in Step 7)
- PermissionPolicyTest: replace guest with admin/member coverage
- AccountPolicyTest, AccountsShopkeeperPolicyTest,
  AccountsInvitationPolicyTest: rename old-tier role refs to member

* Update controller tests for refactored API and 2-tier roles

- ItemTagsControllerTest: switch payloads from queue_number to name;
  drop invalid-format and already_completed cases; rename reset to idle;
  cover the duplicate-name-allowed case
- ShopsControllerTest: drop the reset action test (action removed)
- BaseControllerTest: rewire the validation-error case to use blank name
  instead of duplicate queue_number
- Accounts/AccountsInvitations/AccountsShopkeepers/Permissions/
  RegistrationsControllerTest: rename old-tier role refs to member

* Fix bin/ci: rubocop trailing blank line and brakeman fingerprint refresh

- Drop the extra blank line at end of shops_controller_test.rb class body
  (Layout/EmptyLinesAroundClassBody, leftover from Step 7 reset action removal)
- Refresh brakeman.ignore: AccountsShopkeeper::ROLES collapse changed the
  expanded permit() literal, so the warning fingerprint shifted. Same
  intentional rationale (admin-gated role assignment); only the resolved
  role list and fingerprint differ.

* Documentation cleanup for substrate v2

README.md:
- Drop the 'real-time page updates for Number Tags Webpage' qualifier on Turbo
- Rename 'Number Tags (ItemTags)' to 'Item Tags' in the features list

CLAUDE.md:
- Correct the ItemTag description: name has no uniqueness constraint;
  describe the columns (name/description/position) and binary state instead

docs/openapi.yaml (-90/+31):
- ItemTag schema: drop queue_number/scan_state/customer_read_at/already_completed,
  add name (required)/description/position
- Shop schema: drop scanned_item_tags_count and display_shop_server_path
- /permissions meta: drop maximum_queue_number_length
- Roles: drop senior_manager/junior_manager/senior_member/junior_member/guest,
  add member (5 sites: AccountsShopkeeper attrs/update, AccountsInvitation
  attrs/create/update)
- Endpoints: remove DELETE /shops/{shopId}/reset; rename
  PATCH /item_tags/{itemTagId}/reset → /idle (resetItemTag → idleItemTag)
- ItemTag create/update request bodies switched to name+description+position+state

Also delete:
- docs/pagination-item-tags.md (pre-implementation notes; pagy is now wired in
  the controller, doc is stale)
- app/views/layouts/display.html.erb (orphaned after Step 3 display
  namespace removal — no controllers reference this layout)

* Drop dead queue-length config, controller meta, and test assertion

Step 17 residual sweep caught references the per-step grep missed:
- config/settings.yml: drop maximum_queue_number_length and the
  item_tag.default_count / default_queue_number_length keys (only
  consumed by the removed format validator and create_default_item_tags!)
- PermissionsController#index meta: drop maximum_queue_number_length
  (clients no longer need to enforce the queue-format length cap)
- permissions_controller_test: drop the matching assertion

* Set sample item_tag position to 1

Without an explicit position, the sample row gets NULL, which Postgres
sorts last by default — surprising for a single-row first-run state and
fragile once a client adds more items without managing position
themselves. Setting position 1 gives the sample a definite slot and
keeps the (:position, :name) sort meaningful for clients that don't
implement reordering.

* Remove orphan click_connectedly/click_repeatedly stimulus controllers

Both auto-clicker controllers were used only by the display namespace
queue-board views (now deleted in Step 3). The eager loader in
controllers/index.js needs no change — it just won't find them.
Stimulus itself is now essentially dead weight; defer the wider
JS-stack removal to a follow-up sweep alongside Turbo.

* Auto-assign ItemTag.position to max+1 on create when not provided

Without this, clients that don't manage position end up with NULL
positions on every item, the (shop_id, position) index goes unused,
and ordering between user-added items falls to the :name tiebreaker.

before_create callback no-ops when position was explicitly supplied,
so Shop's create_sample_item_tag (position: 1) and clients that send
their own positions are unaffected. Lighter than acts_as_list (no
gem, no decrement-on-delete machinery — that stays out per phase 1
pitfall #7); this is just a sensible append-to-end default.

* Broaden ShopPolicy to admin || member per collaborative model

The substrate v2 design (overview §2.1 table) grants create_shops,
update_shops, and delete_shops to both admin and member tiers.
Pre-refactor policy was stricter than the design called for:
- create? required owner? (so even non-owner admins couldn't create)
- update? required admin? (so members couldn't update)
- destroy? delegated to create? (so only owner could destroy)

Aligned with the design and with ItemTagPolicy's pattern. Tests
rewritten — two old assertions inverted (non-owner admin can now
create; member can now update), and two read-only true-for-all tests
specialised into admin and member variants for symmetry.

* Add rake task to backfill item_tags.position for existing rows

Assigns sequential position values per shop, ordered by created_at,
starting from current max+1, skipping callbacks/validations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Document SOLID_QUEUE_IN_PUMA=true in .env.sample for dev

Without it, deliver_later jobs (e.g. invitation emails) sit in the
queue unprocessed since no worker runs in development. The Puma
plugin (config/puma.rb) already activates when this env var is set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Remove unused bin/update script

bin/setup covers the dev-environment refresh flow; bin/update was a
legacy Rails generator artifact and was no longer referenced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Add role_redesign:migrate rake task for legacy role data

Fixture-only role redesign (c42249e) left production with stale data:
- accounts_shopkeepers / accounts_invitations jsonb roles still keyed
  on senior_manager, junior_manager, senior_member, junior_member, guest
- orphan rows in roles, permissions, roles_permissions for the dropped
  tiers and queue-specific perms (manage_tags, write_info_to_tags,
  reset_all_tags, complete_or_reset_tags, show_tag_info)

The task folds non-admin keys to member, preserves admin, and deletes
orphan role/permission/roles_permission rows in a single transaction.
Idempotent — safe to re-run.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Remove apple-app-site-association (queue-scan deep link retired)

Substrate v2 drops POST /scan and GET /scan_customer; the only
Universal Link rule in the manifest was /scan/*?item_tag_id=…, which
no longer resolves. No remaining iOS Universal Link routes in v2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Remove Turbo from README features list

API-only Rails app — no Turbo runtime in use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Simplify item_tag not_found message

Drop the "switch organization" hint — Free client v2 has no
Organizations tab, so the guidance is misleading.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Add tests for previously uncovered code paths (#47)

Covers ErrorsController, AdminUser, the static
ShopkeeperAuth::{ConfirmationResults,ResetPasswords} pages,
ShopkeeperMailer, and Shopkeeper::NotificationMailer (invited,
confirmation_instructions, reset_password_instructions).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Refactor: small cleanups across models and controllers (#48)

- ErrorsController: extract shared render_error helper for the
  duplicated JSON/HTML branching
- RegistrationsController: render_*_error hooks now delegate to a
  single private render_resource_error to remove the triple copy
- Account/Shop/ItemTag/AccountsShopkeeper: rename `the_limit_count`
  local to plain `limit`
- AccountsInvitation#set_token: collapse the unique-token loop to
  use `break` as the assignment value
- Shopkeeper: drop two commented-out send_devise_notification lines
  superseded by the custom mailer

Behavior preserved; full test suite passes (393 runs, 792 assertions).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Ignore .claude/scheduled_tasks.lock

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Rate-limit shopkeeper sign-up to 10/IP/3min (#50)

* Rate-limit shopkeeper sign-up to 5/IP/hour

Mass account creation is currently only constrained by the broad
300/IP/5min rack-attack req/ip cap. Add an endpoint-specific limit
on POST /shopkeeper_auth using Rails 8's built-in
ActionController::RateLimiting: 5 requests per IP per hour. When
exceeded, render 429 with a localized JSON error.

Switch the test cache from :null_store to :memory_store so the
limiter's counters can persist within a request sequence
(:null_store no-ops increments). Clear Rails.cache in the standard
test setup so throttle state doesn't leak between tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Tune sign-up rate limit to 10/3min (match jumpstart-pro reference)

Aligns with the reference codebase (jumpstart-pro-rails uses
`to: 10, within: 3.minutes` on user sign-up). The earlier 5/1.hour
was overly strict for legit users — a confused user with bad
password rules or autofill misfires could exhaust the quota and
get locked out for an hour. 10/3min absorbs realistic retry flows
while still constraining bots; per-IP limits are not the primary
defense against rotating-IP attackers anyway.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Fix SessionsController nuking current_platform on header-less sign-in (#49)

* Fix two latent bugs in permissions and sign-in flows

PermissionsController#index compared `confirmed_*_version <
current_*_version`, where `current_version` returns nil if no row is
flagged current. `string < nil` raises ArgumentError → 500. Add a
`version_outdated?` helper that treats a missing current version as
"nothing to update".

ShopkeeperAuth::SessionsController#create unconditionally assigned
`request.headers["source"]` to current_platform and called
`save!(validate: false)`. Sign-ins without the source header
overwrote the user's stored platform with nil, bypassing the
presence/inclusion validation. Now skip the assignment when the
header is blank.

Adds regression tests for both paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Revert PermissionsController nil-version guard

Per review: a missing current PrivacyVersion/TermsVersion is a
server-side data integrity problem (no row published as current).
Crashing loud is preferable to silently telling the client "you're up
to date" — the latter would mask the data issue and mislead clients.

Keeps the SessionsController fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Require 'source' header on shopkeeper sign-in

Per direction: the source header (ios/android) is mandatory. Reject
the request with 401 when missing instead of skipping the
current_platform update — silently signing the user in without the
header would let API tools accumulate sessions that lack platform
attribution and bypass the presence/inclusion validation on Shopkeeper.

Adds the locale key and updates the regression test for the
blank-header path. Updates the existing "no params" test to send the
header so the bad_credentials assertion remains the path under test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Make 'source' header optional on shopkeeper sign-in

Reverts the earlier "require source header" form. Anti-mass-signup
is now handled at the right layer by the sign-up rate_limit
introduced in PR #50, so the sign-in header has no security job
left. current_platform is informational metadata; rejecting
sign-ins on missing metadata is too aggressive — it breaks
non-mobile callers (curl, CI, integration tools, future web client)
without a real benefit.

Skip the current_platform update when the header is blank: the
existing stored value is preserved (instead of being nuked to nil
by the original buggy code path). Drop the missing_source locale
key.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Move login throttles into SessionsController via rate_limit (#51)

Consolidates the rack-attack `logins/ip` and `logins/email`
throttles onto the controller using Rails 8's
ActionController::RateLimiting. Rules now sit next to the action
they guard, mirroring the sign-up `rate_limit` shipped earlier.
The general `req/ip` cap stays in rack-attack since `rate_limit`
is per-controller and middleware is the right layer for an
app-wide flood cap.

The email throttle uses `if: -> { params[:email].present? }` to
preserve the original "skip when no email" behavior — without
this, all email-less requests would share a single counter.

Adds an integration test covering both the IP-keyed throttle
(same IP, different emails) and the email-keyed throttle (same
email, rotating IPs).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Remove dead code: unused gems and locale keys (#52)

Verified-unreferenced removals (every grep below ran across
app/, lib/, config/ and matched only the definition site):

- gem "jbuilder" — no .jbuilder views, no JBuilder usage; all
  responses go through jsonapi-serializer
- gem "inline_svg" — no inline_svg helper calls anywhere
- locale `forgot_your_password` — no callers
- locale `send_me_reset_password_instructions` — no callers
- locale `api.shopkeeper.accounts.owner_required` — no callers
- locale `api.shopkeeper.accounts.admin_required` — no callers

Kept `validate_sign_up_params` / `validate_account_update_params`
overrides in RegistrationsController despite looking unused — the
parent class wires them up via `before_action` (verified in the
gem source).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Normalize NATEMPLATE_API_DOMAIN docs to NATIVEAPPTEMPLATE_API_DOMAIN (#54)

Per docs-private/SUBSTRATE-NAMING-NORMALIZATION.md, all three substrates
should use the canonical NATIVEAPPTEMPLATE_ stem so the agent's existing
NativeAppTemplate -> <slug-pascal> rename pair handles every product-name
occurrence. The Rails repo has no boot/config references to these env
vars - only doc references that point users at the iOS/Android values -
so update them for parity.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Update gems within Gemfile constraints

Routine bundle update: bigdecimal, bootsnap, erb, ffi, irb, json,
minitest, net-imap, nokogiri, pagy, parallel, parser, propshaft,
puma, regexp_parser, rubocop, rubocop-ast, tailwindcss-ruby.
Tests, rubocop, brakeman, bundler-audit all clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant